Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor libraries handling, add handling of descriptors from different sources #89

Merged
merged 7 commits into from
Aug 18, 2020

Conversation

ileasile
Copy link
Contributor

@ileasile ileasile commented Jul 22, 2020

Also, get rid of several singletons in config and replaced
them with factory and free functions.

Fixes #70

@ileasile ileasile requested a review from nikitinas July 22, 2020 08:51
@ileasile ileasile changed the title [Fix #70] Refactor libraries handling, add handling of descriptors from different sources Refactor libraries handling, add handling of descriptors from different sources Jul 22, 2020
@ileasile ileasile force-pushed the library-handling branch 17 times, most recently from 6948c60 to 9dec2e5 Compare July 28, 2020 13:18
@ileasile ileasile force-pushed the library-handling branch 3 times, most recently from b1a5abc to 5d984d9 Compare August 2, 2020 23:24
// Load library descriptor from a remote URL
%use herlib@url[https://site.com/lib.json]
// You may omit library name for file and URL resolution:
%use @file[lib.json]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, the path is relative. Is it relative to the current notebook scope or to notebook parent directory? I think that the later is preferred since one can ship descriptors alongside the notebook.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's relative to current working directory, which is the directory from where you start your jupyter client

Copy link
Contributor

@altavir altavir Aug 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make sense if it would be relative to the notebook file. This way it won't matter where you start the notebook. I mean to use resolveSibling instead of resolve.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I can figure out from this SO answer it's impossible to obtain the notebook path without extending the messaging protocol :( Regarding your offer about resolution against parent directory, I think it's a bit unnatural: can't really figure out the case. Anyway, you may go to the parent directory via .., and it will work

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK then. My idea was to be able to put descriptor in the same directory as the notebook itself and do not think about where do you start notebook from, but it is not so important to waste effort.

@ileasile ileasile force-pushed the library-handling branch 2 times, most recently from 656989e to 6612be5 Compare August 3, 2020 23:05
@ileasile ileasile merged commit 4175906 into master Aug 18, 2020
@ileasile ileasile deleted the library-handling branch August 18, 2020 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add optional version tag to use directive
2 participants